fix(desktop,buzz-acp): log filter matches no target the harness uses - #6035

Thanks — P1 is right, and it is the more interesting half of the bug. Pushed a fix in 0266f52.

I could not read your patch: Complear/buzz returns 404 unauthenticated and 43b516ce8 is not in block/buzz, so the branch link resolves to nothing for me. What follows is my own implementation of your finding rather than a rebase of yours — if the repository was meant to be public, or you can attach the diff, I am happy to compare and take yours if it is better.

Checking the finding against the code before fixing it:

• contains("buzz_acp") is a substring test, so my_buzz_acp=debug bypasses the defaults — correct, and I had not considered it.
• RUST_LOG=buzz_acp=debug is the override anyone debugging this reaches for first, and it kept every other family silenced. The change existed to stop that from happening by default, and left it happening the moment a user tried to look closer, which is worse than the bug it fixes.
• Your point about narrowing is the one that decides the shape. EnvFilter documents a bare level as setting the maximum "for all Spans and Events that are not enabled by other filters", so appending pool=info to a user's RUST_LOG=debug would take their global debug away for five families.

So: a bare global level is now returned exactly as written, and everything else gets the defaults prefixed, with the user's directives appended after. That leans on the documented behaviour that a later directive matching the same target overwrites an earlier one ("the previous directive is overwritten"), which is what keeps an explicit buzz_acp=debug winning over our buzz_acp=info. If that ordering does not hold in practice the failure is mild — the user gets info on the one family they named, and the other five still work — but it is the assumption most worth checking in review, and the one I cannot check myself.

Four tests cover it: an explicit target directive keeps the other families, my_buzz_acp no longer bypasses anything, a bare level survives untouched (including TRACE and whitespace), and an unrelated directive is appended rather than replaced.

The crate side keeps the plain try_from_default_env() contract and is untouched by this, which is a deliberate asymmetry rather than an oversight: the desktop value is a default injected into a child process the user never sees the command line of, whereas someone running buzz-acp from a shell set RUST_LOG themselves and should get exactly what they typed.

Also took your second point and removed the statement count from both durable comments — it was true when I counted it and would quietly stop being true.

Standing caveat, unchanged from the PR description: there is no Rust toolchain on the machine I diagnosed this from, so none of this is built, formatted or linted locally, and the workflows are still sitting at action_required, so CI has not compiled it either. To be precise about what your run does and does not cover — it exercised your patch on top of ffa51d84, so this code has never executed anywhere. If a maintainer approves the workflows that resolves itself; until then, treat every claim above as reasoning about the code rather than an observation of it.

Updating the record, since the PR description and my earlier comment both say this is unbuilt, unformatted and unlinted — that is no longer accurate, and a reviewer skimming the thread should not have to work that out.

Per @themiguelamador's re-review of 0266f52: the full buzz-acp suite (778 unit + 9 lifecycle), the five Tauri filter tests added here, strict all-target Clippy for both buzz-acp and the Desktop Tauri workspace, and cargo fmt / git diff --check all pass. That covers exactly the three things I flagged as unverifiable from my side, and it settles the one assumption I called out as worth checking — that a later directive for the same target keeps precedence over the default we prefix.

To be precise about whose evidence this is: that is a contributor's local run, not this repository's CI. The workflows are still at action_required, so nothing in block/buzz has compiled this branch. I have no Rust toolchain here, so I cannot independently reproduce either figure.

Nothing further from me — the branch is ready as far as anyone without write access can take it.

🤖 Request changes at exact head 0266f52cc2382253e8987c9156050c3a7a58401e.

The observability goal is valid: buzz_acp=info does not cover the crate’s explicit acp::*, pool::*, canvas::*, engram::*, and observer targets, so useful diagnostics can disappear. The current implementation broadens logging across sensitive content and does not reliably control the final Desktop child environment.

1. Default logging now persists conversation and command content

The new default includes acp=info and pool=info (desktop/src-tauri/src/managed_agents/runtime.rs:927-964; duplicated in crates/buzz-acp/src/lib.rs:66-81).

Those are broad target-prefix directives, so they enable existing content-bearing events:

• every model agent_message_chunk is logged verbatim at info under acp::stream (crates/buzz-acp/src/acp.rs:1748-1759);
• full slash-command text and arguments are logged at info under pool::prompt (crates/buzz-acp/src/pool.rs:2088-2095; extraction retains the remainder in queue.rs:927-1003);
• tool titles, IDs, and advertised command names are agent-controlled and logged under acp::tool / acp::update (acp.rs:1762-1805).

Desktop redirects the child’s stdout/stderr into a persistent runtime log. The 10 MB rotation check runs only when the file is opened, not while a long-lived process appends (desktop/src-tauri/src/managed_agents/storage.rs:637-660).

There is also an integrity consequence. Untrusted fields are not newline-framed or structurally encoded. Desktop later scans physical lines for prefixes such as Agent reported error:, llm auth:, and llm model not found: and promotes matches into runtime failure state (storage.rs:868-904; runtime/lifecycle.rs:84-103). Model/command text containing a newline plus one of those prefixes can therefore imitate a real diagnostic record.

This is confirmed local plaintext persistence and error-state spoofing, not a demonstrated relay/network leak.

Proposed solution

Use a metadata-first default:

1. Enable family-wide warnings/errors, then enable only audited lifecycle/metadata targets at info.
2. Keep raw-content targets such as acp::stream, acp::thought, wire payloads, and raw prompt/command arguments off by default.
3. At info level, log bounded metadata only: byte count, stable command name, tool kind/status, IDs, durations. Full bodies/arguments should require a deliberate documented opt-in.
4. Emit structured records with escaped fields and have Desktop accept a failure only from a structured diagnostic target/code. Do not infer trusted errors from arbitrary physical line prefixes.
5. Bound/rotate the active file while the child is running, not only on the next open.

A minimal target exclusion is acceptable only if it is backed by a complete target audit and an executable sentinel test; changing the content-bearing call sites to metadata is safer against future filter changes.

Acceptance tests

• Under the default filter, unique sentinels placed in assistant text, slash-command arguments, tool titles, and command names do not appear in the runtime log.
• An explicit content opt-in exposes only the intended content target.
• A payload containing \nAgent reported error: ... cannot create a recognized AgentLogError.
• A long streaming process cannot grow the active log beyond the documented bound.
• Raising or broadening the default to re-enable any raw-content target fails the policy test.

2. Saved Desktop RUST_LOG overwrites the computed filter

The PR computes and writes RUST_LOG at runtime.rs:530. Desktop later writes descriptor.env at runtime.rs:814-824.

descriptor.env is the final layered map:

harness definition -> global -> persona -> individual agent

It is intentionally written last, and RUST_LOG is not reserved (reserved_env_keys.rs:28-76). A supported saved value such as RUST_LOG=buzz_acp=debug therefore replaces the new value from line 530 and hides the other five families again.

The current tests exercise only child_rust_log_filter_from; they do not inspect the final child Command after environment layering.

Proposed solution

Resolve this once at the final child-process boundary:

1. finish computing descriptor.env;
2. obtain the effective configured RUST_LOG using the existing definition/global/persona/agent precedence;
3. prevent the generic environment loop from writing RUST_LOG;
4. merge that final value with the safe policy;
5. call command.env("RUST_LOG", ...) exactly once.

Do not combine the ambient Desktop value early and then allow a saved value to overwrite it later.

Acceptance tests

Inspect the final spawned-child environment for:

• no configured value;
• harness-definition value;
• global value;
• persona value;
• per-agent value;
• conflicting values proving the existing precedence;
• explicit target overrides preserving their intended specificity.

Removing the final merge or reintroducing the early write must fail these tests.

3. Valid numeric global levels change meaning

BARE_LEVELS recognizes only off/error/warn/info/debug/trace (runtime.rs:938-957). The locked tracing-subscriber parser also accepts numeric global levels 0..5.

Executable evidence at this head:

direct RUST_LOG=0  -> no tracing
merged LOG_FILTER,0 -> family info appears

direct RUST_LOG=5  -> trace/debug output appears
merged LOG_FILTER,5 -> family targets are narrowed to info

Thus 0 is widened and 5 is narrowed, violating explicit user intent. The 0 case also defeats a user’s request to disable logging.

Proposed solution

Use tracing_subscriber::EnvFilter parsing as the source of truth instead of a partial word list. Preserve valid global directives exactly, including numeric forms; merge only target-specific configurations. Invalid values should take the documented fallback path.

Add parser/executable coverage for named levels, 0..5, whitespace/case, target directives, mixed directives, empty input, and invalid input.

4. Direct/self-hosted fallback has no causal coverage

Desktop and direct buzz-acp launches contain separate copies of the default. Removing observer=info from Desktop fails the new Desktop test. Removing it from crates/buzz-acp/src/lib.rs:77 leaves the test graph green; scoped search in crates/buzz-acp/src finds no test that owns that constant.

Proposed solution

Prefer one shared filter/policy builder consumed by both launch paths. If dependency boundaries require two constants, add a direct buzz-acp executable/family test that proves the unset/invalid fallback covers every intended target and that content targets remain disabled. Mutating either launch path must fail.

Verification already completed

• Full buzz-acp: 787 passed, 0 failed.
• Full Desktop Tauri: 2,546 passed, 16 ignored, 0 failed.
• Format, check, and strict Clippy gates passed.
• Real executable cases covered unset, named-global, target-specific, unrelated, invalid, and numeric filters.
• Mutation checks caught Desktop family removal, order reversal, and named-global breakage; the direct fallback mutation survived.
• Worktrees were restored clean and git diff --check passed.

The green suites establish a sound baseline, but they do not cover the production and privacy boundaries above. Keep the expanded diagnostics goal, make the default metadata-only, resolve RUST_LOG after all environment layering with the real parser, and give both launch paths causal ownership before merge.

Reworked on 6427b8d, rebased onto current main. Thank you — the content point is the one I had not weighed at all, and it is the one that mattered.

1. Content in the default — you are right, and the default is now warn.

I had argued the change was safe because the debug targets stay off, which is a statement about volume. Content is a different property and I never checked it: acp::stream logs the model's reply verbatim at info, and the desktop appends the child's output to a plaintext file. My default would have persisted every conversation on every machine running an agent.

The five families now default to warn rather than info:

buzz_acp=info,acp=warn,pool=warn,canvas=warn,engram=warn,observer=warn

That keeps what the change exists for — the error and warn statements an owner currently cannot see anywhere — while enabling no content-bearing event at all. I checked each of the 42 warn/error sites: they carry timeouts, channel and session ids, and failure strings, not assistant text or command arguments. Reading content back remains possible as a deliberate RUST_LOG opt-in, which is the distinction you drew.

This is narrower than your metadata-first proposal and does not touch the 87 call sites. If you would rather have the call sites changed to bounded metadata, that is a bigger change and I would want a maintainer to want it before writing it.

2. Resolution order — fixed. RUST_LOG is now written once, after descriptor.env has been applied, next to apply_effort_env, which is post-loop for exactly this reason. A saved buzz_acp=debug merges with the defaults instead of replacing them. You were pointing at a mechanism I had documented myself on #2698 and then failed to connect to my own patch.

3. Numeric levels — fixed. 0–5 are recognised as global levels alongside the names, so RUST_LOG=0 is no longer widened into our defaults and 5 is no longer narrowed. Covered by a test.

4. Two copies — improved, not solved. The desktop copy now lives in runtime/log_filter.rs with its own tests, including a policy test that fails if any family is raised to info. The crate-side constant is still a second literal with only a "keep in sync" comment: the desktop does not depend on buzz-acp, so a shared builder needs a dependency edge or a third crate, and I did not want to make that call inside a logging fix. Your mutation observation stands — deleting a family from crates/buzz-acp/src/lib.rs still leaves the tests green.

What I did not do, deliberately. No end-to-end sentinel test through a real spawn, no bound on the active log while a child appends, and no structured framing for the Agent reported error: promotion path. The last two are properties of the existing desktop log pipeline rather than something this change introduces — and with content off by default this change no longer feeds them. They look worth their own issue; I did not want to grow a filter fix into a logging redesign.

CI, actually run this time. I had no Rust toolchain here, which is why earlier notes carried an "unbuilt" caveat. Rather than keep guessing I ran this repository's own CI on a fork, where it needs no approval: two runs on the reworked branch.

• Rust Lint failed on the first run with two cargo fmt diffs — a chain that wraps and a double blank line left by the module move. Both were mine, both are fixed in 5079b90, and the second run is green.
• Desktop Core passes: the file-size ratchet that failed the previous head is satisfied, with runtime.rs and its test file back at their upstream line counts.
• Security passes after the rebase; the earlier failure was RUSTSEC-2026-0258 in h2, fixed on main by fix: bump h2 for RUSTSEC-2026-0258 #6222.
• Unit Tests, Windows Rust (msvc), Desktop Build (macOS), Desktop E2E Relay, Relay E2E, Backend Integration and both cross-compiles pass — 19 green jobs on the second run.

One red remains, and it is not this change: Desktop Smoke E2E (4) fails on tooltip-semantics.spec.ts:109, the same test failing on upstream main right now (run 32443163358). Separately, mentions.spec.ts:323 failed on the first run and passed on the second with only formatting changed between them, so that one is a flake rather than a regression.

Happy to be told the fork-CI evidence is not good enough and to wait for a maintainer to approve the workflows here instead.

Rebased onto current main (b9bebd0) — #6501 landed a new function in lib.rs at the same spot as the constant here, so the conflict was additive and both are kept.

Re-ran this repository's CI on the rebased branch: 23 jobs green, none failing, including Rust Lint, Desktop Core, Security, Unit Tests, Windows Rust (msvc), the macOS build and all four smoke shards. The two E2E failures I reported in my previous comment did not recur — consistent with them being flakes rather than anything from this change.

Nothing outstanding from my side. Worth noting that this branch touches files that move often, so it has already gone CONFLICTING twice in a week; I will keep rebasing while it is open, but the value of the change is in the default filter rather than in the branch staying alive, so if a maintainer would rather take the one-line constant change directly, that is an entirely fine outcome. If you do take it that way rather than merging, a Co-authored-by: Artem Trofymenko <99894081+artemtrofymenko@users.noreply.github.com> trailer would be appreciated — the commits here already carry that authorship, and it keeps the change traceable back to the measurements in #2698 that produced it.

Independent confirmation from a production journal, plus two things that have changed since this PR last moved.

The defect is live, and the cost is measurable

Read crates/buzz-acp/src at 1c8321cd0. There are 93 tracing statements that declare their own target, across 20 distinct target roots, and not one of those roots begins with buzz_acp. All 93 are production sites. None sit inside a mod tests.

Both ends of the filter agree on the same string. EnvFilter::new("buzz_acp=info") at lib.rs:2435 is the crate fallback, and child_rust_log_filter() at desktop/src-tauri/src/managed_agents/runtime/metadata.rs:80 is what the desktop actually sets on the child at runtime.rs:531. So an operator does not get to opt out of this by accident.

Here is what it costs on one Windows box running a dozen managed agents. 17 per-agent log files, spanning 2026-08-15 to 2026-09-02, 51,472 lines carrying a parseable target.

Zero. Not "a few". Eighteen days of a busy fleet.

The zero is a measurement, not a broken grep

The remaining 847 lines come from targets like buzz_agent, buzz_agent::handoff, buzz_agent::llm, buzz_agent::mcp and serve_inner. Those are the child process writing into the same files under its own filter, and they land fine. 50,625 plus 847 is exactly 51,472, so nothing is unaccounted for. The file is perfectly capable of holding a non-buzz_acp target. The harness's own 93 statements are dropped at the filter, before they ever reach it.

What it looks like when it bites you

I did not go looking for a logging bug. I was trying to answer a much narrower question, which is whether permission_mode actually reaches the session.

On main it does. pool.rs:1522-1526 applies the mode when the agent advertises it in session/new, and apply_permission_mode logs the outcome. All 313 startup banners in these files report permission_mode=bypassPermissions, so is_default() is false and that branch is not being skipped for the boring reason.

I still cannot tell you whether the mode was applied or silently skipped. Every outcome on that path, the success info, the application-level warn and the fatal error, is emitted at pool::permission. All three are invisible. The one branch that is genuinely silent in the code, the agent not advertising the mode, is indistinguishable from the two that are supposed to be loud. That is the whole value of this PR in one example.

Two things to flag

This PR is CONFLICTING against current main. Head b9bebd09, polled twice a few seconds apart to be sure it was not a stale computation. It last moved on 22 August.

#3309 fixes the same defect by the same mechanism, re-rooting the targets under buzz_acp. It is also CONFLICTING, at head 6fdeba54. One commit, dated 28 July, and the API reports zero reviews, zero review comments and zero issue comments on it. Two open PRs against one defect, both stalled on conflicts, and the defect is still shipping in the meantime. Worth someone picking one and landing it.

I am not opening a third. This is evidence, not a competing proposal.

@cristiansotogarciaxatech thank you — the 18-day journal measurement is the strongest evidence this defect has, and the pointer to #3309 is the more useful half.

I did not find #3309 before opening this, and I should have: I searched pull requests keyed on child_rust_log_filter, the symbol my fix touches, and #3309 does not touch it — it re-roots the targets in the crate instead. Searching by the symbol of your own solution finds only people who chose your solution.

#3309 is the better fix and I am standing down in its favour. It corrects the defect at the source, so both the crate fallback and the desktop's child filter work unchanged, and it repairs the operator path this PR cannot: RUST_LOG=buzz_acp=debug from TESTING.md starts working, and a value saved per-agent in the UI stops re-silencing the other families — which was the substance of @ravarora2's second finding here.

I have left the one blocking observation on that PR: the rename carries acp::stream to buzz_acp::acp::stream at info, which switches verbatim model replies into a persistent plaintext log by default — the same objection @ravarora2 raised here, and the reason this PR now defaults those families to warn.

Happy to close this as a duplicate on a maintainer's word. If any of it is worth salvaging afterwards it is only the numeric-level handling in the merge helper (RUST_LOG=0 was being widened, 5 narrowed), and that is a small follow-up on top of #3309 rather than a reason to keep a second PR open. This branch is CONFLICTING again in any case — child_rust_log_filter moved to runtime/metadata.rs on main — and I would rather rebase once, onto whichever approach survives, than twice.

Thanks for the straight answer, and for going and reading #3309 properly rather than defending your own patch. I think the call is right.

I put the detail on #3309 rather than duplicate it here. Two parts of it bear on your decision.

Your blocker holds for acp::stream, and the pool::prompt half is worse than you wrote it. extract_slash_command returns rest.to_string() at queue.rs:1154, so the command field carries the whole message after leading mentions are stripped, not only the arguments. The rest of the list is smaller than feared. acp::thought sits at debug (acp.rs:1789), and nine of the eleven acp::wire sites are debug including every one that dumps a JSON payload, so the default filter never reaches them. That makes the content fix on #3309 two sites rather than a level redesign.

On the salvage. That behaviour is a live defect on main today and it survives #3309 landing, so it is a real follow-up and not a reason to keep this PR open. child_rust_log_filter appends buzz_acp=info to any non-empty filter that does not already mention buzz_acp, so RUST_LOG=off still logs buzz_acp at info, and RUST_LOG=trace quietly narrows buzz_acp to info. Neither is what the operator asked for.

Your read on the conflict is correct. child_rust_log_filter now lives at desktop/src-tauri/src/managed_agents/runtime/metadata.rs:114 and is called from runtime.rs:573, both at c6ca9d94.

Closing as a duplicate of #3309, which fixes this defect at the source by re-rooting the targets rather than widening the filter — older, smaller, and it repairs the operator path (RUST_LOG=buzz_acp=debug from TESTING.md) that a filter-side patch cannot.

The one part of this PR that #3309 does not address is the desktop's own RUST_LOG construction, where a bare global level is overridden in both directions. That is now #7287, standing alone, with the credit to @cristiansotogarciaxatech who identified it here.

The content blocker discussed on this PR applies to #3309 as well and is recorded there: after the rename, buzz_acp=info reaches buzz_acp::acp::stream, which logs the model's reply verbatim into a persistent plaintext log.